Skip to content

feat: Add the FDv2 data system and expose it through configuration#310

Open
kinyoklion wants to merge 14 commits into
mainfrom
rlamb/sdk-2186/fdv2-data-system
Open

feat: Add the FDv2 data system and expose it through configuration#310
kinyoklion wants to merge 14 commits into
mainfrom
rlamb/sdk-2186/fdv2-data-system

Conversation

@kinyoklion

@kinyoklion kinyoklion commented Jun 17, 2026

Copy link
Copy Markdown
Member

What this adds

The piece that ties the FDv2 building blocks together and turns them on: FDv2DataSystem, the manager routing that applies FDv2 payloads, and the configuration that opts in. This completes the FDv2 work in common_client — everything below the public Flutter API.

  • FDv2DataSystem composes the data source factories the DataSourceManager consumes. It owns the state that must outlive any single orchestrator: the current selector and the context it belongs to. A fresh orchestrator is built per connection-mode switch and per identify; the selector survives mode switches (initializers are skipped when a selector is already held) but resets when the context changes, since a selector is specific to one context. buildFactories() returns factories for streaming, polling, and background — offline has no data source and is handled by the manager directly. Custom connection modes from config replace the built-in definitions by name.
  • DataSourceManager payload routing. The PayloadEvent case — a no-op placeholder in the orchestrator PR — now applies the change set through handlePayload and completes the pending identify, the same way DataEvent does for FDv1.
  • Configuration & exposure. DataSystemConfig (providing it, even empty, opts into FDv2); LDCommonConfig.dataSystem; LDCommonClient constructs an FDv2DataSystem and installs its factories when dataSystem is configured, otherwise the FDv1 sources run unchanged; and the new public types are exported.

When dataSystem is absent the FDv1 path is byte-for-byte unchanged, so existing behavior is unaffected.

Testing

  • DataSourceManager test: a PayloadEvent is applied through handlePayload, marks the source valid, and completes identify (a dropped/no-op payload would leave identify hanging — this distinguishes the real routing from the placeholder).
  • FDv2DataSystem tests: buildFactories exposes streaming/polling/background and not offline; each factory builds a fresh data source per call; a custom connection mode replaces the built-in definition.
  • DataSystemConfig default (no custom modes).
  • Full common_client suite passes; flutter_client_sdk (the dependent package) analyzes and tests clean against these changes.
  • End-to-end behavior of the wired data system is exercised by the FDv2 contract tests, which land with the contract-service changes in a later PR (and pass on the integration branch today).

SDK-2186


Note

Medium Risk
Changes core flag acquisition wiring when dataSystem is set and alters identify completion for payload events; default behavior is unchanged when config is omitted, but the new path touches initialization and data-source orchestration.

Overview
Adds an EAP opt-in for FDv2: setting LDCommonConfig.dataSystem (even const DataSystemConfig()) switches startup to FDv2DataSystem factories instead of the existing FDv1 streaming/polling/background wiring; leaving dataSystem null keeps the prior path.

DataSystemConfig exposes ConnectionModeId (streaming/polling/background) and optional connectionModes overrides that replace built-in ModeDefinition pipelines. FDv2DataSystem builds per-mode factories (fresh orchestrator per connection), persists selector across mode switches but resets it on context change, and skips initializers when a selector is already held.

DataSourceManager now routes PayloadEvent through handlePayload and shares _completeIdentify with FDv1 DataEvent handling so identify completes when FDv2 payloads land. Public exports add the new config and mode-definition types.

Tests cover payload→identify completion and FDv2DataSystem factory/override behavior.

Reviewed by Cursor Bugbot for commit 6225462. Bugbot is set up for automated code reviews on this repo. Configure here.

Comment thread packages/common_client/lib/src/config/data_system_config.dart Outdated
@kinyoklion kinyoklion force-pushed the rlamb/sdk-2186/fdv2-data-system branch 3 times, most recently from 9e8ec33 to fff23d4 Compare June 17, 2026 17:54
Comment thread packages/common_client/lib/src/config/data_system_config.dart Outdated
@kinyoklion kinyoklion force-pushed the rlamb/sdk-2186/fdv2-data-system branch from fff23d4 to 6225462 Compare June 17, 2026 18:00
@kinyoklion kinyoklion marked this pull request as ready for review June 17, 2026 21:53
@kinyoklion kinyoklion requested a review from a team as a code owner June 17, 2026 21:53
Comment thread packages/common_client/test/data_sources/fdv2/data_system_test.dart Outdated
Comment thread packages/common_client/lib/src/config/data_system_config.dart
Comment thread packages/common_client/lib/src/data_sources/fdv2/data_system.dart Outdated
Comment thread packages/common_client/lib/src/data_sources/fdv2/data_system.dart Outdated
@kinyoklion kinyoklion marked this pull request as draft June 18, 2026 19:44
…ata source

Split the identify strategy into FDv1/FDv2 data managers so cache handling
lives with each protocol. FDv2 no longer loads the cache at identify; the
data system reads it through a real cachedFlagsReader and feeds it into the
pipeline. A PayloadEvent basis flag, set by the orchestrator, gates both the
valid status (moved out of handlePayload) and wait-for-network resolution, so
cached flags apply without reporting a live connection. Offline is now a real
pipeline mode (cache initializer, no synchronizer) that loads cache while the
manager keeps the offline status. Adds ConnectionModeId.offline and
FlagManager.readCached.
…ctory

The held selector was discarded inside the data source factory by comparing
context instance identity, which only held while the factory was invoked for
every context change. Move the reset to the data manager's identify path,
keyed on the context canonical key, so a context change clears the basis
regardless of the active connection mode (including offline) and independent
of factory invocation timing. The factory no longer tracks the context.
The override test asserted only that a DataSource was produced, which holds
whether or not the override is honored. Assert that resolvedDefinition picks
the override for the overridden mode and the built-in otherwise. Translating a
definition's entries into concrete sources stays covered by the entry-factory
tests. Adds a visibleForTesting accessor and the meta dependency it needs.
Flutter 3.22.3 pins meta to 1.12.0 through its SDK dependency (via
connectivity_plus), so a ^1.16.0 floor broke workspace version solving on
that toolchain while passing on the newer one. visibleForTesting has been in
meta since well before 1.12.0.
…and selector

Replaces the derived PayloadEvent.basis flag with the explicit signals it
stood for. A cache load -- a full changeset with no selector -- is applied and
resolves a cached identify, but does not mark the source valid or satisfy a
wait-for-network identify. Network data carries a server selector and does
both. An intent-none, or being offline, resolves a wait-for-network identify
since no selector will arrive. This mirrors js-core's FDv2DataManagerBase
minimum-data-availability handling. The wire-level basis query parameter is
unaffected.
…ntext keys

Identify now unconditionally clears the held selector, matching js-core's
FDv2DataManagerBase (which resets the selector on every identify). The
previous context-key comparison split the "which context is the selector
for" invariant between the data manager (holding the key) and the data
system (holding the selector), with nothing keeping the two consistent. Mode
switches still keep the selector, since they reach the data source manager
directly rather than through the data manager.
The note described the method by contrast to the FDv1 verbs and to a
prematurely-valid bug that no longer exists, rather than the code as it is.
Drops the contrast with the removed context-instance inference and the stale
"reset on a context change" wording; the data manager clears the selector on
every identify.
It returns without completing when a wait-for-network identify has not yet
seen fresh data, so the name should reflect that it is conditional.
setValid was gated on the change set carrying a selector, but an intent-none
(the server confirming no changes) carries none -- so a healthy reconnect that
reported no changes left the status stuck at interrupted. Gate status (and the
wait-for-network identify) on whether the change set is server data -- a
selector-bearing payload or an intent-none -- rather than a cache load (a
selector-less full). Matches js-core and android, where any synchronizer
response, including no-change, restores valid. Adds a regression test.
Three linked changes that finish the FDv2 data system in common_client:

- Status: the manager marks the source valid on any applied change set while
  online (offline keeps its offline status). No selector inference -- a
  cache load and a no-change response both report valid, matching js-core.

- Initialization: the orchestrator, which knows the source tier, emits a
  single InitializedEvent (selector-bearing initializer, initializer
  exhaustion with data or in a cache-only system, or the first synchronizer
  change set). The manager resolves a wait-for-network identify on it; a
  cached identify resolves earlier on the first applied payload. Replaces the
  fragile type+selector inference, and an FDv1-fallback first response now
  completes a fresh identify.

- FDv1 fallback: a new FDv1 fallback synchronizer (an FDv1 poller whose
  responses translate to full, selector-less change sets tagged
  fdv1Fallback:false) is appended as the terminal, initially-blocked slot
  when a mode configures it. The orchestrator ignores a fallback directive
  once already on FDv1. The x-ld-fd-fallback detection was already present.
A fresh fallback synchronizer instance must not inherit a prior instance's
ETag, which would let it receive a 304 for a connection it never made and
keep stale data. The if-none-match header is per-requestor instance state.
@kinyoklion kinyoklion marked this pull request as ready for review June 23, 2026 21:12
@kinyoklion kinyoklion requested a review from tanderson-ld June 24, 2026 21:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants